Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix websocket handshake for ROS Noetic #31

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

Tezozomoc47
Copy link

I added a ClientConfigurator implementation to the ClientEndpoint annotation of the Ros class.
This Configurator can modify the Http headers before sending or after receiving.
Even only adding this component made the "Origin" header compatible to rosbridge.

@aslakjo
Copy link

aslakjo commented Mar 17, 2022

I have seen the issue of failing hadshake similar to RobotWebTools/rosbridge_suite#508 However we are useing Melodic.

Also tested with this pull request and verified that it fixes our issues. Hope it can be merged in 👍

@kavikode
Copy link

In ROS Noetic, I'm having this same issue and I set websocket_external_port = 20003

Still I get the same issue as follows:

2022-03-21 06:25:23-0400 [-] failing WebSocket opening handshake ('port 9090 in HTTP Host header 'localhost:9090' does not match server listening port 20003')
2022-03-21 06:25:23-0400 [-] dropping connection to peer tcp4:127.0.0.1:43906 with abort=False: port 9090 in HTTP Host header 'localhost:9090' does not match server listening port 20003

How do I fix this issue? Thanks in advance.

@aslakjo
Copy link

aslakjo commented Mar 21, 2022

From what we saw, the jrosbridge has an error in its use of websockets this pull request implements a fix for this. This in turn fixes the handshake. We did not need to use the external port only this pull request.

@kavikode
Copy link

kavikode commented Mar 21, 2022

Thank you. @aslakjo can you please explain how to do a pull request to fix this error as I am new?

Also, how do I update the launch file?

I really appreciate your response.

@aslakjo
Copy link

aslakjo commented Mar 21, 2022

We did bump the version number and performed a local build and pushed it to our local artifactory. However i am hoping for a merge so that we can skip the local artifactory and rely on a build through maven central. This must be done by @rctoris

@kavikode
Copy link

Thanks for letting me know and I will be awaiting your response.

@aslakjo
Copy link

aslakjo commented Apr 5, 2022

I still hope we can have a updated version with this PR @rctoris

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants